Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unbreak setting of entrypoint in exec form when property mode is enabled #1020

Merged
merged 1 commit into from
May 16, 2018

Conversation

stromnet
Copy link
Contributor

@stromnet stromnet commented May 9, 2018

Any <entrypoint><exec><arg>...</arg> would disappear when any property mode was enabled (but corresponding property not defined as property), due to bad string conversion.

Fix is to get rid of all string conversions dealing with Arguments (which was mostly from test).

Bug in #948

@codecov
Copy link

codecov bot commented May 9, 2018

Codecov Report

Merging #1020 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #1020      +/-   ##
============================================
+ Coverage     51.78%   51.82%   +0.04%     
- Complexity     1369     1372       +3     
============================================
  Files           147      147              
  Lines          7479     7477       -2     
  Branches       1135     1134       -1     
============================================
+ Hits           3873     3875       +2     
+ Misses         3241     3237       -4     
  Partials        365      365
Impacted Files Coverage Δ Complexity Δ
...ic8/maven/docker/config/RunImageConfiguration.java 91.48% <ø> (-0.88%) 47 <0> (ø)
.../docker/config/handler/property/ValueProvider.java 89.24% <100%> (+0.35%) 11 <1> (+1) ⬆️
...config/handler/property/PropertyConfigHandler.java 72.59% <100%> (+0.69%) 109 <1> (ø) ⬇️
...8/maven/docker/config/BuildImageConfiguration.java 82.2% <100%> (ø) 60 <0> (ø) ⬇️
.../maven/docker/config/HealthCheckConfiguration.java 78.94% <100%> (ø) 16 <0> (ø) ⬇️
...java/io/fabric8/maven/docker/config/Arguments.java 60.46% <0%> (+6.97%) 7% <0%> (+2%) ⬆️

@rhuss
Copy link
Collaborator

rhuss commented May 16, 2018

There are currently conflicts because of another merge for health check.
Also, I think its a good point in time now to switch to Java 8 and lambdas (instead of using Function). Wdyt ?

Any <entrypoint><exec><arg>...</arg> would disapsear due to bad string conversion.
Fix is to get rid of all string conversions dealing with Arguments
(which was mostly from test).
@stromnet stromnet force-pushed the unbreak-arguments-override branch from 0d90e9f to a9da3b6 Compare May 16, 2018 08:07
@stromnet
Copy link
Contributor Author

Rebased on master, ready for merge.

I'd rather merge this one before putting effort into changing into Java8, as this bug was a blocker for one of our projects. But yes, that's yet another use-case where lambdas would be nice :)

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's go with it. I will open an issue that we should streamline the configuration with lambdas and Optional to avoid the many raw case stuff.

Copy link
Collaborator

@rhuss rhuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's go with it. I will open an issue that we should streamline the configuration with lambdas and Optional to avoid the many raw case stuff.

@rhuss rhuss merged commit 04d728f into fabric8io:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants